Colocated final#8
Conversation
Signed-off-by: SamitHuang <285365963@qq.com>
Signed-off-by: samithuang <285365963@qq.com>
Add rollout backend client and test qwen2.5-0.5b non-colocate training
Signed-off-by: samithuang <285365963@qq.com>
Eliminate intermediate CPU tensors
Reorder weight synchronization support for colocate and non-colocate scenarios in the goal plan.
* Draft router design Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com> * Add vllm router Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com> * Add router to script Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com> * Fix gpu memory utilization Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com> * Fix output token ids Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com> * Add more nccl flag Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com> * Fix bug Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com> --------- Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com>
Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com>
Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com>
Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com>
Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com>
Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com>
Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com>
Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com>
Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com>
Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com>
Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com>
Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com>
Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com>
Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com>
Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com>
Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com>
Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com>
Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request integrates vLLM as a rollout backend for Slime, introducing a backend-agnostic architecture with dedicated adapters and a translation sidecar. The changes include refactoring rollout orchestration, generalizing argument names, and decoupling weight synchronization from SGLang-specific dependencies. Feedback identifies several issues, including a hardcoded log path in the vLLM engine, a debugging leftover that overrides rollout offloading settings, and a logic error in connection tracking for request cancellation. Additionally, improvements are suggested for parallelizing remote calls during weight updates and handling pipe closures in the NCCL bridge subprocess.
| try: | ||
| ray.get(engine.set_weight_version.remote(self.weight_version)) | ||
| except Exception as exc: | ||
| logger.warning("Failed to set weight version on engine: %s", exc) | ||
|
|
There was a problem hiding this comment.
Calling ray.get() inside a loop for each engine serializes the remote calls, which can significantly slow down weight synchronization when multiple engines are used. It is more efficient to collect all remote references and wait for them in parallel.
| try: | |
| ray.get(engine.set_weight_version.remote(self.weight_version)) | |
| except Exception as exc: | |
| logger.warning("Failed to set weight version on engine: %s", exc) | |
| version_refs = [ | |
| engine.set_weight_version.remote(self.weight_version) | |
| for engine in self._colocated_engines | |
| ] | |
| if version_refs: | |
| try: | |
| ray.get(version_refs) | |
| except Exception as exc: | |
| logger.warning("Failed to set weight version on engines: %s", exc) |
| cmd = conn.recv() | ||
| if cmd is None: |
There was a problem hiding this comment.
The conn.recv() call in the bridge subprocess should handle EOFError. This ensures that the subprocess exits cleanly if the parent process terminates unexpectedly and the pipe is closed.
| cmd = conn.recv() | |
| if cmd is None: | |
| while True: | |
| try: | |
| cmd = conn.recv() | |
| except EOFError: | |
| break |
| resp = await self._client.post(url, json=vllm_payload) | ||
| self._active_connections.add(resp) |
There was a problem hiding this comment.
The resp object is added to _active_connections after the request has already completed (await self._client.post(...)). This makes the tracking ineffective for cancelling in-flight requests during an abort_request call. If connection closing is intended as an abort mechanism, the request needs to be tracked while it is active (e.g., by tracking the asyncio.Task).
Signed-off-by: knlnguyen1802 <knlnguyen1802@gmail.com>
2 participants
No description provided.